-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CT-853] Generate order replacement protos #1551
Conversation
WalkthroughThe recent updates enhance the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
protocol/indexer/events/events.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
protocol/indexer/off_chain_updates/types/off_chain_updates.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
Files selected for processing (4)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (8 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (12 hunks)
- proto/dydxprotocol/indexer/events/events.proto (2 hunks)
- proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (2 hunks)
Additional Context Used
Path-based Instructions (4)
proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.proto/dydxprotocol/indexer/events/events.proto (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (9)
proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (3)
24-44
: The newOrderPlacementStatus
enum is well-documented and covers various scenarios of order placement. Ensure that all systems using this enum are updated to handle the new values correctly.
102-108
: The update to theOffChainUpdateV1
message to includeOrderReplaceV1
is crucial for supporting new order replacement features. Confirm that the Kafka consumers are updated to parse this new field.
91-97
: The addition of theOrderReplaceV1
message integrates well with the existing structure. It's important to verify that all components that interact with order messages can handle this new type.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (3)
Line range hint
11-42
: The TypeScript enumOrderPlacementStatus
and its SDK counterpart are updated to reflect the new order placement statuses. Ensure that all TypeScript code handling these enums is updated accordingly.
Line range hint
281-311
: The addition ofOrderReplaceV1
and its SDK type in the TypeScript definitions is well done. This will facilitate the handling of order replacements in the TypeScript part of the system.
68-99
: The functionsorderPlacementStatusFromJSON
andorderPlacementStatusToJSON
are updated to handle the new enum values. It's important to ensure that all parts of the system that serialize or deserialize these values are aware of these changes.proto/dydxprotocol/indexer/events/events.proto (1)
Line range hint
250-285
: The addition ofStatefulOrderReplacementV1
to theStatefulOrderEventV1
message is a significant update. It's crucial to ensure that all systems processing these events can handle the neworder_replace
field correctly.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (2)
627-627
: The addition oforderReplace
fields in bothStatefulOrderEventV1
andStatefulOrderEventV1SDKType
interfaces aligns with the PR's objective to handle order replacements. Ensure that the corresponding logic to handle these new fields is implemented wherever these events are processed.Also applies to: 642-642
716-718
: Introduction ofStatefulOrderEventV1_StatefulOrderReplacementV1
andStatefulOrderEventV1_StatefulOrderReplacementV1SDKType
interfaces. This is a crucial addition for supporting the new order replacement functionality. It's important to ensure that theorder
field within these interfaces is properly utilized in the order processing logic to maintain the integrity of order replacements.Also applies to: 721-723
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
protocol/indexer/events/events.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
protocol/indexer/off_chain_updates/types/off_chain_updates.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
Files selected for processing (4)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (8 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (7 hunks)
- proto/dydxprotocol/indexer/events/events.proto (2 hunks)
- proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts
- proto/dydxprotocol/indexer/events/events.proto
- proto/dydxprotocol/indexer/off_chain_updates/off_chain_updates.proto
Additional Context Used
Path-based Instructions (1)
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (4)
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/off_chain_updates/off_chain_updates.ts (4)
283-294
: The newly added interfacesOrderReplaceV1
andOrderReplaceV1SDKType
are consistent with the existing pattern of interface definitions in the file.
Line range hint
601-631
: The integration ofOrderReplaceV1
into theOffChainUpdateV1
encoding and decoding functions is done correctly. This ensures that the new order replacement functionality is properly serialized and deserialized in the system's messaging framework.
513-575
: The encoding and decoding functions forOrderReplaceV1
are implemented correctly following the established patterns in this file. It's important to ensure that theorder
field, which is optional, is handled correctly in all scenarios whereOrderReplaceV1
is used.Verification successful
The search results indicate that
OrderReplaceV1
is used in various places within theoff_chain_updates.ts
file. The encoding, decoding, and fromPartial functions handle the optionalorder
field correctly by checking if it isundefined
before processing it.
- Encoding: The
encode
function checks ifmessage.order
isundefined
before encoding it.- Decoding: The
decode
function assignsmessage.order
only if the corresponding tag is present.- fromPartial: The
fromPartial
function checks ifobject.order
isundefined
before assigning it tomessage.order
.These implementations ensure that the optional
order
field is handled correctly in all scenarios whereOrderReplaceV1
is used.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify correct handling of the optional `order` field in `OrderReplaceV1`. # Test: Search for usage of `OrderReplaceV1`. Expect: Correct handling of the optional `order` field. rg --type typescript $'OrderReplaceV1'Length of output: 72
Script:
#!/bin/bash # Description: Verify correct handling of the optional `order` field in `OrderReplaceV1`. # Test: Search for usage of `OrderReplaceV1`. Expect: Correct handling of the optional `order` field. rg --type ts 'OrderReplaceV1'Length of output: 2304
304-304
: The addition oforderReplace
toOffChainUpdateV1
andOffChainUpdateV1SDKType
integrates the new order replacement functionality into the existing message structure sent to the Indexer. Ensure that all systems consuming these messages are updated to handle the neworderReplace
field.Also applies to: 315-315
@Mergifyio backport release/indexer/v5.x |
✅ Backports have been created
|
* Generate order replacement protos (cherry picked from commit 722ec24) # Conflicts: # protocol/indexer/events/events.pb.go # protocol/indexer/off_chain_updates/types/off_chain_updates.pb.go
* Generate order replacement protos
* Generate order replacement protos
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
StatefulOrderEventV1
with enhanced functionalities.OrderReplaceV1
with new encoding and decoding capabilities.Improvements
OrderPlaceV1
andOrderReplaceV1
with updated status types for improved order management.